Skip to content

fix(signals): compare symbol keys in UNSTABLE_MEMO_OUTPUT - #3775

Merged
ryansolid merged 1 commit into
nextfrom
fix/unstable-memo-symbol-keys
Oct 5, 2026
Merged

ryansolid merged 1 commit into
nextfrom
fix/unstable-memo-symbol-keys

Conversation

@ryansolid

Copy link
Copy Markdown
Member

Summary

UNSTABLE_MEMO_OUTPUT compared plain objects by Object.keys, which skips symbol keys. A memo that returns a fresh symbol-keyed box each run therefore looked like a run of empty objects and was reported as "new-but-equivalent", even though every box held a different value.

dynamic in @solidjs/web returns { [FLIGHT]: promise } from its factory memo while a result is in flight, so every refetch through dynamic counted toward the warning. Found in the server todos example: toggling rows warned on <Todos> › computed, which is the dynamic(() => getTodoList(...)) factory. Each refetch boxes a new promise, so it was a false positive.

shallowEquivalent now compares Reflect.ownKeys, so symbol keys are included. Objects whose symbol-keyed values are identical still warn.

Public API changes

  • Diagnostic behaviour: UNSTABLE_MEMO_OUTPUT now counts symbol-keyed (and non-enumerable) own properties when deciding whether two plain objects are equivalent. Plain objects that differ only in a symbol-keyed value no longer warn. No exports, options or diagnostic codes change.

Verification

  • New test in packages/signals/tests/attribution-unstable-memo.test.ts: one memo whose symbol-keyed value changes every run stays quiet, and one whose symbol-keyed value is always the same object still warns. It fails before the fix.
  • Full @solidjs/signals suite passes, and the source type-check (tsconfig.build.json) is clean.
  • In the server todos example, with this change built in, the <Todos> › computed warning no longer appears. The remaining useSubmissions warnings there are fixed separately by fix: useSubmissions keeps the same list while its submissions are unchanged solid-router#653.

@changeset-bot

changeset-bot Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 2b3079a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 12 packages
Name Type
@solidjs/signals Patch
test-integration Patch
@solidjs/web Patch
@solidjs/babel-plugin Patch
@solidjs/compiler Patch
@solidjs/diagnostics Patch
@solidjs/element Patch
@solidjs/h Patch
@solidjs/html Patch
solid-js Patch
@solidjs/universal Patch
todos-server-example Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions

github-actions Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Size (brotli, eager entry chunk)

scenario head vs base cap lazy chunks (not counted)
signals: core floor (createSignal/Memo/Effect/Root/flush) 7.32 KB 0 B 7.33 KB ✅
signals: + createStore 14.51 KB 0 B 14.53 KB ✅
signals: + isPending/latest 9.45 KB 0 B 9.45 KB ✅
app: render + one signal (the simple-app floor) 9.81 KB 0 B 9.83 KB ✅
app: hydrating (no stores) with Show/For/Loading/Errored/lazy 17.65 KB 0 B 17.66 KB ✅ lazy-page.js 0.04 KB
app: hydrating + every store primitive family 28.79 KB 0 B 28.80 KB ✅ lazy-page.js 0.04 KB
app: CSR with Show/For/Loading/Errored/lazy 12.81 KB 0 B 12.82 KB ✅ lazy-page.js 0.04 KB
app: CSR, observe tier (same app on the observe artifacts) 14.39 KB 0 B 14.39 KB ✅ lazy-page.js 0.04 KB
app: CSR, observe tier + attribution engine enabled 28.59 KB +2 B (+0.0%) 28.61 KB ✅ lazy-page.js 0.04 KB
frames: eager client consumer (frames client + transport, lazy codec) 13.77 KB 0 B 13.78 KB ✅
page: base server components (hydrating + dynamic + frames + sf reference) 44.76 KB 0 B 44.78 KB ✅ decode.js 6.07 KB, lazy-page.js 0.04 KB
page: live server components (base + live/GET + action + isPending/latest) 48.44 KB 0 B 48.45 KB ✅ decode.js 6.07 KB, lazy-page.js 0.04 KB
server: floor (getRequestEvent + isServer) 1.33 KB 0 B 1.34 KB ✅
server: renderToString (the server-render floor) 20.41 KB 0 B 20.42 KB ✅

Bundled with Rolldown (what Vite ships), brotli q11, decimal KB. Caps in scripts/size/scenarios.js; the floor and page caps in floor-caps.json are frozen (lower only, or Size-Exception: in the PR body).

@coveralls

coveralls commented Oct 4, 2026 •

Copy link
Copy Markdown

Coverage Report for CI Build 37281019340

Coverage remained the same at 75.991%

Details

  • Coverage remained the same as the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 1195
Covered Lines: 962
Line Coverage: 80.5%
Relevant Branches: 925
Covered Branches: 649
Branch Coverage: 70.16%
Branches in Coverage %: Yes
Coverage Strength: 27.68 hits per line

💛 - Coveralls

@codspeed

codspeed Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 188 untouched benchmarks


Comparing fix/unstable-memo-symbol-keys (2b3079a) with next (203ab1a)

Open in CodSpeed

The unstable-output check compared plain objects by Object.keys, which
skips symbol keys. A memo that returns a fresh symbol-keyed box each run
looked like a run of empty objects, so it warned as new-but-equivalent
although every box held a different value. web's dynamic does this for
an in-flight promise ({ [FLIGHT]: promise }), so every refetch through
dynamic counted toward the warning.

shallowEquivalent now compares Reflect.ownKeys. Boxes whose symbol-keyed
values are identical still warn.

Co-authored-by: Claude via Cursor <noreply@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@ryansolid
ryansolid force-pushed the fix/unstable-memo-symbol-keys branch from eec54bf to 2b3079a Compare October 5, 2026 08:00
@ryansolid
ryansolid merged commit 5622be8 into next Oct 5, 2026
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants